VT-d: fix iommu_domid for PCI/PCIx devices assignment

Currently, it clears iommu_domid and domid_map at the end of
domain_context_unmap_one() if no other devices under the same iommu
owned by this domain. But, when assign a PCI/PCIx device to a guest,
it also assigns its upstream bridge to the guest, and they use the
same iommu_domid. In the deassignment, the iommu_domid and domid_map
are cleared in domain_context_unmap_one() for the assigned PCI/PCIx
device, therefore it cannot get valid iommu_domid in followed
domain_context_unmap_one for its upstream bridge. It causes PCI/PCIx
device re-assignment failure.

This patch moves the iommu_domid and domid_map clearing code to the
end of domain_context_unmap, where all dependent
domain_context_unmap_one()s are completed, thus fix above issue.

Signed-off-by: Weidong Han <Weidong.han@intel.com>

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 5fa2e4cc52b3fb1398b09661f2037fd08154be6a..15027d459135c2da5c5b316418dfa55a68bd4730 100644 (file)
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1351,9 +1351,6 @@ static int domain_context_unmap_one(
     struct context_entry *context, *context_entries;
     u64 maddr;
     int iommu_domid;
-    struct pci_dev *pdev;
-    struct acpi_drhd_unit *drhd;
-    int found = 0;
 
     ASSERT(spin_is_locked(&pcidevs_lock));
     spin_lock(&iommu->lock);
@@ -1391,34 +1388,6 @@ static int domain_context_unmap_one(
         iommu_flush_iotlb_dsi(iommu, iommu_domid, 0, flush_dev_iotlb);
     }
 
-
-    /*
-     * if no other devices under the same iommu owned by this domain,
-     * clear iommu in iommu_bitmap and clear domain_id in domid_bitmp
-     */
-    for_each_pdev ( domain, pdev )
-    {
-        if ( pdev->bus == bus && pdev->devfn == devfn )
-            continue;
-
-        drhd = acpi_find_matched_drhd_unit(pdev);
-        if ( drhd && drhd->iommu == iommu )
-        {
-            found = 1;
-            break;
-        }
-    }
-
-    if ( found == 0 )
-    {
-        struct hvm_iommu *hd = domain_hvm_iommu(domain);
-
-        clear_bit(iommu->index, &hd->iommu_bitmap);
-
-        clear_bit(iommu_domid, iommu->domid_bitmap);
-        iommu->domid_map[iommu_domid] = 0;
-    }
-
     spin_unlock(&iommu->lock);
     unmap_vtd_domain_page(context_entries);
 
@@ -1428,16 +1397,19 @@ static int domain_context_unmap_one(
 static int domain_context_unmap(struct domain *domain, u8 bus, u8 devfn)
 {
     struct acpi_drhd_unit *drhd;
+    struct iommu *iommu;
     int ret = 0;
     u32 type;
-    u8 secbus;
+    u8 tmp_bus, tmp_devfn, secbus;
     struct pci_dev *pdev = pci_get_pdev(bus, devfn);
+    int found = 0;
 
     BUG_ON(!pdev);
 
     drhd = acpi_find_matched_drhd_unit(pdev);
     if ( !drhd )
         return -ENODEV;
+    iommu = drhd->iommu;
 
     type = pdev_type(bus, devfn);
     switch ( type )
@@ -1445,37 +1417,39 @@ static int domain_context_unmap(struct domain *domain, u8 bus, u8 devfn)
     case DEV_TYPE_PCIe_BRIDGE:
     case DEV_TYPE_PCIe2PCI_BRIDGE:
     case DEV_TYPE_LEGACY_PCI_BRIDGE:
-        break;
+        goto out;
 
     case DEV_TYPE_PCIe_ENDPOINT:
         gdprintk(XENLOG_INFO VTDPREFIX,
                  "domain_context_unmap:PCIe: bdf = %x:%x.%x\n",
                  bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
-        ret = domain_context_unmap_one(domain, drhd->iommu, bus, devfn);
+        ret = domain_context_unmap_one(domain, iommu, bus, devfn);
         break;
 
     case DEV_TYPE_PCI:
         gdprintk(XENLOG_INFO VTDPREFIX,
                  "domain_context_unmap:PCI: bdf = %x:%x.%x\n",
                  bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
-        ret = domain_context_unmap_one(domain, drhd->iommu, bus, devfn);
+        ret = domain_context_unmap_one(domain, iommu, bus, devfn);
         if ( ret )
             break;
 
-        if ( find_upstream_bridge(&bus, &devfn, &secbus) < 1 )
+        tmp_bus = bus;
+        tmp_devfn = devfn;
+        if ( find_upstream_bridge(&tmp_bus, &tmp_devfn, &secbus) < 1 )
             break;
 
         /* PCIe to PCI/PCIx bridge */
-        if ( pdev_type(bus, devfn) == DEV_TYPE_PCIe2PCI_BRIDGE )
+        if ( pdev_type(tmp_bus, tmp_devfn) == DEV_TYPE_PCIe2PCI_BRIDGE )
         {
-            ret = domain_context_unmap_one(domain, drhd->iommu, bus, devfn);
+            ret = domain_context_unmap_one(domain, iommu, tmp_bus, tmp_devfn);
             if ( ret )
                 return ret;
 
-            ret = domain_context_unmap_one(domain, drhd->iommu, secbus, 0);
+            ret = domain_context_unmap_one(domain, iommu, secbus, 0);
         }
         else /* Legacy PCI bridge */
-            ret = domain_context_unmap_one(domain, drhd->iommu, bus, devfn);
+            ret = domain_context_unmap_one(domain, iommu, tmp_bus, tmp_devfn);
 
         break;
 
@@ -1484,9 +1458,45 @@ static int domain_context_unmap(struct domain *domain, u8 bus, u8 devfn)
                  "domain_context_unmap:unknown type: bdf = %x:%x.%x\n",
                  bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
         ret = -EINVAL;
-        break;
+        goto out;
+    }
+
+    /*
+     * if no other devices under the same iommu owned by this domain,
+     * clear iommu in iommu_bitmap and clear domain_id in domid_bitmp
+     */
+    for_each_pdev ( domain, pdev )
+    {
+        if ( pdev->bus == bus && pdev->devfn == devfn )
+            continue;
+
+        drhd = acpi_find_matched_drhd_unit(pdev);
+        if ( drhd && drhd->iommu == iommu )
+        {
+            found = 1;
+            break;
+        }
+    }
+
+    if ( found == 0 )
+    {
+        struct hvm_iommu *hd = domain_hvm_iommu(domain);
+        int iommu_domid;
+
+        clear_bit(iommu->index, &hd->iommu_bitmap);
+
+        iommu_domid = domain_iommu_domid(domain, iommu);
+        if ( iommu_domid == -1 )
+        {
+            ret = -EINVAL;
+            goto out;
+        }
+
+        clear_bit(iommu_domid, iommu->domid_bitmap);
+        iommu->domid_map[iommu_domid] = 0;
     }
 
+out:
     return ret;
 }